Skip to content

Fix issues found during the connection-reuse review - #20

Merged
jonathan-kosgei merged 7 commits into
masterfrom
fix-noticed-issues
Nov 3, 2026
Merged

jonathan-kosgei merged 7 commits into
masterfrom
fix-noticed-issues

Conversation

@jonathan-kosgei

@jonathan-kosgei jonathan-kosgei commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Follow-up to #19: fixes the items that review listed under "Noticed, not changed". The test helpers decode HTTP bodies as UTF-8 explicitly so the suite passes under any locale, the Language model gains the code attribute the API sends, the deprecated Feign calls are replaced, the README table of contents, heading level and sample output are corrected, the two Lombok delegate getters are named after what they return, the unused polyglot-groovy build extension is removed, and the open Dependabot alert (Guava) is fixed. No public symbol is renamed or removed; the version number is unchanged (0.3.0).

Changes

  • Test helpers decode as UTF-8 (TestContext.get()/post(), MockIpdataServer.handleBulk(), MappingTest): on a JVM whose default charset is not UTF-8 (JDK 11 under a POSIX locale) 11 tests failed on emoji flags and currency symbols; the suite now passes under any locale. TestContext.post() also sends its body as UTF-8, and the mock server uses StandardCharsets.UTF_8 for its response bytes and query-string decoding.
  • languages[].code is mapped: new Language.code() (null when the API does not send it). Before, the lenient client mapper dropped the value silently and the strict test mapper would have rejected a fixture carrying it. The test fixtures now carry the attribute and the new LanguageTest covers the full model, getFields(ip, IpdataField.LANGUAGES), the absent value and the strict mapper.
  • Deprecated Feign calls replaced: Ipv6SafeClient uses Request.create(..., RequestTemplate), which also keeps the RequestTemplate on the rebuilt request instead of dropping it; FieldDecoder and ApiErrorDecoder call Response.Body.asReader(StandardCharsets.UTF_8) instead of the deprecated asReader() (UTF-8 underneath, so same behaviour). The main sources compile without deprecation warnings.
  • README: the table of contents links "API" to #api (the #create-an-instance anchor did not exist); "Bulk data" is a level-4 heading under API like its siblings; the "Basic Usage" sample output is regenerated from the model (it predated carrier, is_satellite, is_anycast, languages[].code, threat.is_vpn, threat.blocklists and threat.scores); languages[].code is listed with the other recently added fields.
  • IpdataServiceSupport delegate getters: getApi() now returns the full-model client and getSingleFieldClient() the single-field client. Both exist only as sources for Lombok's @Delegate on a package-private class and are now both private: no generated method and no public API changes.
  • Unused build extension removed: .mvn/extensions.xml declared io.takari.polyglot:polyglot-groovy 0.4.3, which only matters for a pom.groovy; it was downloaded on every fresh build and never used.
  • Dependabot alert: the open high-severity alert is GHSA-xxph-c9ww-hj94 (CVE-2026-102554, uncontrolled memory allocation when deserializing CompactHashMap, CompactHashSet or MapMakerInternalMap) on guava 33.4.8-jre; bumped to 33.7.2-jre, the first patched version. In the same commit Jackson becomes a direct dependency (jackson-databind 2.22.4, jackson-annotations 2.22, released once per minor line since 2.20): the code compiles against it directly but only received it through feign-jackson, which pins jackson-databind 2.13.4 (CVE-2022-42003; its jackson-core 2.13.4: CVE-2025-52999). The two calls these versions deprecate, ObjectMapper.setSerializationInclusion(Include) and CacheBuilder.expireAfterWrite(long, TimeUnit), are replaced by their equivalents setDefaultPropertyInclusion(Include) and expireAfterWrite(Duration).

Not changed

  • Scores and Blocklist keep their plain getX() getters: switching them to fluent accessors like the other models would rename public methods, and adding a second set of accessors would duplicate the API.

Verification

All local commands with JDK 11 (JAVA_HOME=/usr/lib/jvm/java-11-openjdk-amd64) and the Maven wrapper (3.6.3); the CI command is mvn clean test post-integration-test on temurin 11.

  1. Baseline on master (e574984), UTF-8 locale (LC_ALL=C.UTF-8): ./mvnw -B -Dmaven.artifact.threads=1 clean test post-integration-test → Tests run: 80, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.
  2. Baseline on master, POSIX locale (LC_ALL=POSIX LANG=POSIX, JVM file.encoding = ANSI_X3.4-1968): same command → Tests run: 80, Failures: 11, Errors: 0, Skipped: 0, BUILD FAILURE (BulkTest 4, FullModelTest 4, MultipleFieldsSelectionTest 2, NewResponseFieldsTest.modelRoundTripsTheNewFields 1; all "JSON documents are different" on the mangled strings).
  3. This branch (7c91a05), UTF-8 locale: ./mvnw -B -Dmaven.artifact.threads=1 -Dmaven.compiler.showDeprecation=true -Dmaven.compiler.showWarnings=true clean test post-integration-test → Tests run: 84, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS, no javac deprecation warning (on master the same flags report Request.create in Ipv6SafeClient and asReader() in FieldDecoder and ApiErrorDecoder).
  4. This branch, POSIX locale: ./mvnw -B -Dmaven.artifact.threads=1 clean test post-integration-test → Tests run: 84, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.
  5. ./mvnw dependency:tree: jackson-databind:2.22.4, jackson-core:2.22.4, jackson-annotations:2.22, guava:33.7.2-jre; every other dependency unchanged.
  6. README sample: src/test/resources/io/ipdata/client/fixtures/1.1.1.1.json deserialized into IpdataModel and serialized back with a snake_case mapper that keeps null values, keys reordered to the API's documented order.
  7. CI (.github/workflows/ci.yml, pull_request trigger) for 7c91a05: run Claude/verify all tests g2 mnu #12, https://github.com/ipdata/java/actions/runs/41642065187, job test success on the first attempt (mvn clean test post-integration-test, temurin 11, ubuntu-latest). No follow-up commits were needed.

🤖 Generated with Claude Code

TestContext.get()/post() and MockIpdataServer.handleBulk() wrapped HTTP
bodies in an InputStreamReader without a charset, and MappingTest read
fixture.json the same way, so on a JVM whose default charset is not
UTF-8 (JDK 11 under a POSIX locale) the emoji flags and currency symbols
served by the mock came back mangled and 11 tests failed. All of these
now decode with StandardCharsets.UTF_8 explicitly; TestContext.post()
sends its body as UTF-8 too, and the mock server uses the same constant
for its response bytes and for query-string decoding.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The API returns a code for every entry of languages[] but the Language
model had no field for it: the lenient client mapper silently dropped
the value and the strict test mapper would have rejected any fixture
carrying it. Add Language.code() (null when the API does not send it),
add the attribute to the test fixtures and cover it with LanguageTest
(full model, field selection, absent value, strict mapping).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ipv6SafeClient rebuilt the request with the deprecated
Request.create(HttpMethod, String, Map, byte[], Charset), which also
dropped the RequestTemplate; it now uses the overload that takes the
template. FieldDecoder and ApiErrorDecoder read bodies through the
deprecated Response.Body.asReader(), which is UTF-8 underneath; they now
pass StandardCharsets.UTF_8 explicitly. Same behaviour, and the main
sources compile without deprecation warnings.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
getSingleFieldClient() returned the full-model client and getApi() the
single-field client. Both exist only to feed Lombok's @DeleGate on a
package-private class, so the names are swapped and both getters made
private; no generated method and no public API changes.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
.mvn/extensions.xml declared io.takari.polyglot:polyglot-groovy 0.4.3,
which lets Maven read a pom.groovy. The project uses a plain pom.xml, so
the extension and its Groovy runtime were downloaded on every fresh
build and never used.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The table of contents linked "API" to #create-an-instance, an anchor
that does not exist; "Bulk data" was a level-3 heading although it
belongs to the API section; the "Basic Usage" sample output predated
carrier, is_satellite, is_anycast, languages[].code, threat.is_vpn,
threat.blocklists and threat.scores. The sample is now the model
serialized from the 1.1.1.1 test fixture, and languages[].code is listed
with the other recently added fields.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Guava 33.4.8 is affected by GHSA-xxph-c9ww-hj94 (CVE-2026-102554:
uncontrolled memory allocation when deserializing CompactHashMap,
CompactHashSet or MapMakerInternalMap); 33.7.2 is the first patched
version. The project compiles against Jackson directly (ObjectMapper,
@JsonProperty) but only received it through feign-jackson, which pins
jackson-databind 2.13.4, affected by CVE-2022-42003 (and jackson-core
2.13.4 by CVE-2025-52999). jackson-databind 2.22.4 and
jackson-annotations 2.22 (annotations are released once per minor line
since 2.20) are now declared as direct dependencies, which also makes
them visible to the repository's dependency graph.

The new versions deprecate two calls in IpdataServiceBuilder; they are
replaced by their documented equivalents, available since Jackson 2.7
and Guava 25: ObjectMapper.setDefaultPropertyInclusion(Include) for
setSerializationInclusion(Include), and
CacheBuilder.expireAfterWrite(Duration) for expireAfterWrite(long,
TimeUnit).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@jonathan-kosgei
jonathan-kosgei merged commit f8feb0e into master Nov 3, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants